Skip to content

feat(binding-opcua): Add channel security support #1401#1415

Merged
relu91 merged 9 commits into
eclipse-thingweb:masterfrom
node-opcua:feat/binding_opcua_set_security_1401
Sep 22, 2025
Merged

feat(binding-opcua): Add channel security support #1401#1415
relu91 merged 9 commits into
eclipse-thingweb:masterfrom
node-opcua:feat/binding_opcua_set_security_1401

Conversation

@erossignon
Copy link
Copy Markdown
Contributor

@erossignon erossignon commented Sep 2, 2025 •
edited
Loading

Implements OPC UA channel security by allowing the configuration of security mode and policy and authentication

This change introduces a certificate manager to handle client-side certificates.

Key changes:

  • Added new SecurityScheme
     OPCUASecureSecuritySchemeBase (abstract)
         -> OPCUAChannelSecurityScheme
	       -> OPCUAUnsecureChannelScheme
	       -> OPCUASecureSecurityScheme
	 -> OPCUASecureSecurityScheme 
               -> OPCUACertificateAuthenticationScheme 
               -> OPCUAUserNameAuthenticationScheme

  • Implemented a shared OPCUACertificateManager for PKI.

    PKI folder for OPCUAPrococolClient is set to env-path("binding-opcua-wot").PKI

    which resolve to
    on window:
    C:\Users<User>\AppData\Roaming\binding-opcua-node-wot\Config\PKI
    on linux:
    ~/.config/binding-opcua-node-wot/PKI on linux'
    on macOs:
    ~/Library/Application Support/binding-opcua-node-wot/PKI

  • Added tests for secure communication.

@erossignon erossignon mentioned this pull request Sep 2, 2025
Closed
@erossignon
Copy link
Copy Markdown
Contributor Author

erossignon commented Sep 2, 2025

Waiting for #1417 to be completed

@erossignon erossignon force-pushed the feat/binding_opcua_set_security_1401 branch 2 times, most recently from 7acf786 to cce16ef Compare September 2, 2025 16:19
danielpeintner
danielpeintner reviewed Sep 12, 2025
View reviewed changes
Comment thread packages/core/src/thing-description.ts Outdated
@erossignon
feat(binding-opcua): Add channel security support eclipse-thingweb#1401
8ae1c34 
Implements OPC UA channel security by allowing the configuration of
security mode and policy and authentication

This change introduces a certificate manager to handle client-side certificates.

Key changes:
- Added new SecurityScheme

     OPCUASecureSecuritySchemeBase (abstract)
         -> OPCUAChannelSecurityScheme
	       -> OPCUAUnsecureChannelScheme
	       -> OPCUASecureSecurityScheme
	 -> OPCUASecureSecurityScheme
	       -> OPCUACertificateAuthenticationScheme
	       -> OPCUAUserNameAuthenticationScheme

- Implemented a shared `OPCUACertificateManager` for PKI.

  PKI folder for OPCUAPrococolClient is set to
    env-path("binding-opcua-wot").PKI

    which resolve to
        on window:
	  C:\Users\<User>\AppData\Roaming\binding-opcua-node-wot\Config\PKI
	on linux:
	  ~/.config/binding-opcua-node-wot/PKI on linux'
	on macOs:
	  ~/Library/Application Support/binding-opcua-node-wot/PKI

- Added tests for secure communication.
@erossignon erossignon force-pushed the feat/binding_opcua_set_security_1401 branch from cce16ef to 2e190cb Compare September 12, 2025 13:45
@erossignon
feat(binding-opcua): add method to reflect on current connected user …
7a2546e 
…authenticated and security in demo server
@erossignon
feat(binding-opcua): improve channel security test
8d979c4 
  - add ability to call the WhoIAm method
    to loopback on the actual credentials settings
    of the connected user,
    This ensure that the security scheme has worked
    as expected.
@erossignon
chore(binding-opcua): relax settings in .mocharc.yml
82b0299
@erossignon
chore(binding-opcua) eclipse-thingweb#1401 use uav: prefix for new op…
8158bb8 
…cua security schemes
@erossignon
chore(core): eclipse-thingweb#1401 use uav: prefix for new opcua secu…
250be6e 
…rity schemes used in tests
@erossignon erossignon force-pushed the feat/binding_opcua_set_security_1401 branch from 2e190cb to 250be6e Compare September 12, 2025 14:17
@relu91
Copy link
Copy Markdown
Member

relu91 commented Sep 15, 2025

PR looks good, but I have these two questions/suggestions:

  • ClientManager logic can be factored out in a single ts file/class
  • Are we all ok to expose and add OPCUA specific typings and test to the core package (which should be agnostic from the underlying protocol)?

danielpeintner
danielpeintner reviewed Sep 15, 2025
View reviewed changes
Comment thread packages/binding-opcua/src/opcua-protocol-client.ts
@danielpeintner
Copy link
Copy Markdown
Member

danielpeintner commented Sep 15, 2025

  • Are we all ok to expose and add OPCUA specific typings and test to the core package (which should be agnostic from the underlying protocol)?

Ideally, we should not do that.
I wonder whether we can create a generic placeholder which each security scheme could use...

@erossignon
Copy link
Copy Markdown
Contributor Author

erossignon commented Sep 15, 2025 •
edited
Loading

  • Are we all ok to expose and add OPCUA specific typings and test to the core package (which should be agnostic from the underlying protocol)?

@relu91, Can you clarify what you have in mind ?

@relu91
Copy link
Copy Markdown
Member

relu91 commented Sep 19, 2025

two things:

  • Move the new declarations about security schemes to opcua package
  • Make the tests about the combo scheme generic. In the sense that I don't think there is a need to use specific OPC UA security bindings to test the new logic introduced in the core (or do I miss something?).

p.s. I noticed that SecurityType is not used anywhere in our codebase I'd deprecate it and remove it in 1.0.0 @danielpeintner

@danielpeintner danielpeintner mentioned this pull request Sep 19, 2025
Closed
@danielpeintner
Copy link
Copy Markdown
Member

danielpeintner commented Sep 19, 2025

p.s. I noticed that SecurityType is not used anywhere in our codebase I'd deprecate it and remove it in 1.0.0 @danielpeintner

👍 Makes sense
Created #1419

@erossignon
Copy link
Copy Markdown
Contributor Author

erossignon commented Sep 19, 2025

Created #1419

I can take it while I am here.

danielpeintner
danielpeintner approved these changes Sep 22, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@danielpeintner danielpeintner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@relu91
Copy link
Copy Markdown
Member

relu91 commented Sep 22, 2025

Maybe I'm wrong, but I think you missed this:

ClientManager logic can be factored out in a single ts file/class

@erossignon
Copy link
Copy Markdown
Contributor Author

erossignon commented Sep 22, 2025 •
edited
Loading

Maybe I'm wrong, but I think you missed this:

ClientManager logic can be factored out in a single ts file/class

@relu91 Sorry Can you be more specific ?

update:

I guess, by extrapolating the meaning of "ClientManager" that you wan't the singleton management for the OPCACertificateManager to be factored out of the OpcuaProtocolClient class.
I'll do this.

I'll also move the resolution of WotSecurity=>OPCUASecurity to a dedicated file.

Let me know if there are more you had in mind.

@erossignon erossignon force-pushed the feat/binding_opcua_set_security_1401 branch from 4c643cd to 01a1a56 Compare September 22, 2025 14:13
@erossignon
refactor(binding-opcua): move cert mgt and security resolution
269ea39 
  move certificate management and OPCUA security resolution to
  own files, for clarity.

  improve OCPUA Certificate manager singleton lifecycle
@erossignon erossignon force-pushed the feat/binding_opcua_set_security_1401 branch from 01a1a56 to 269ea39 Compare September 22, 2025 15:11
relu91
relu91 approved these changes Sep 22, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@relu91 relu91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I had in mind, thank you.

@relu91
Copy link
Copy Markdown
Member

relu91 commented Sep 22, 2025

Ah, now I noticed a little bit of inconsistency in the file naming scheme, sometimes using _ sometime - . We can fix it later with a refector PR.

@relu91 relu91 merged commit 813dee6 into eclipse-thingweb:master Sep 22, 2025
14 checks passed
@relu91 relu91 mentioned this pull request Sep 22, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@relu91 relu91 relu91 approved these changes

@danielpeintner danielpeintner danielpeintner approved these changes

@JKRhb JKRhb Awaiting requested review from JKRhb JKRhb is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@erossignon @relu91 @danielpeintner